Skip to content

Conversation

@filipcirtog
Copy link
Collaborator

@filipcirtog filipcirtog commented Dec 22, 2025

Description

This PR implements support for the new GCP port-based architecture, which uses port mapping to reduce resource provisioning. Unlike the legacy architecture that requires dedicated resources for each Atlas node, the new design uses a single set of resources to support up to 1000 nodes through port mapping, enabling direct targeting of specific nodes using only one customer IP address.

Changes Overview

New Attributes

  1. port_mapping_enabled (Optional, Computed)

    • Added to mongodbatlas_privatelink_endpoint resource (Optional, settable)
    • Added to mongodbatlas_privatelink_endpoint data source (Computed, read-only)
    • Added to mongodbatlas_privatelink_endpoint_service resource (Computed, read-only)
    • Added to mongodbatlas_privatelink_endpoint_service data source (Computed, read-only)
    • When set to true on the endpoint resource, enables the new port-based architecture for GCP
    • Defaults to false (legacy architecture)
  2. gcp_endpoint_status (Computed)

    • Added to mongodbatlas_privatelink_endpoint_service resource and data source
    • Status of the individual GCP endpoint
    • Only populated for the new port-based architecture (when port_mapping_enabled = true)
    • Returns one of: INITIATING, AVAILABLE, FAILED, DELETING

Reused Attributes

  1. private_endpoint_ip_address (Optional)

    • Previously used for Azure, Now also used for the new GCP port-based architecture
    • For new architecture: Required and should be the IP address of the forwarding rule
    • For legacy architecture: Not used
    • Conflicts with endpoints attribute
  2. endpoint_service_id (Required)

    • For legacy architecture: Can be any identifier string
    • For new architecture: Should be the forwarding rule name
  • Updated documentation for all affected resources and data sources

  • Added test coverage:

    • Test case for explicitly enabled port mapping
    • Test case for explicitly disabled port mapping

Link to any related issue(s): CLOUDP-363082

Type of change:

  • Bug fix (non-breaking change which fixes an issue). Please, add the "bug" label to the PR.
  • New feature (non-breaking change which adds functionality). Please, add the "enhancement" label to the PR. A migration guide must be created or updated if the new feature will go in a major version.
  • Breaking change (fix or feature that would cause existing functionality to not work as expected). Please, add the "breaking change" label to the PR. A migration guide must be created or updated.
  • This change requires a documentation update
  • Documentation fix/enhancement

Required Checklist:

  • I have signed the MongoDB CLA
  • I have read the contributing guides
  • I have checked that this change does not generate any credentials and that they are NOT accidentally logged anywhere.
  • I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
  • I have added any necessary documentation (if appropriate)
  • I have run make fmt and formatted my code
  • If changes include deprecations or removals I have added appropriate changelog entries.
  • If changes include removal or addition of 3rd party GitHub actions, I updated our internal document. Reach out to the APIx Integration slack channel to get access to the internal document.

Further comments

@filipcirtog filipcirtog changed the title Cloudp 363082 gcp port based routing implementation feat: Add port_mapping_enabled attribute to privateEndpoint and privateEndpointService Dec 22, 2025
@filipcirtog filipcirtog changed the title feat: Add port_mapping_enabled attribute to privateEndpoint and privateEndpointService feat: Add port_mapping_enabled attribute to privatelink_endpoint and privatelink_endpoint_service Dec 22, 2025
@filipcirtog filipcirtog changed the title feat: Add port_mapping_enabled attribute to privatelink_endpoint and privatelink_endpoint_service feat: Adds port_mapping_enabled attribute to privatelink_endpoint and privatelink_endpoint_service Dec 22, 2025
@filipcirtog filipcirtog marked this pull request as ready for review December 22, 2025 15:19
@filipcirtog filipcirtog requested review from a team as code owners December 22, 2025 15:19
Copilot AI review requested due to automatic review settings December 22, 2025 15:19
@github-actions
Copy link
Contributor

APIx bot: a message has been sent to Docs Slack channel

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds support for GCP Private Service Connect (PSC) port-mapping architecture by introducing a port_mapping_enabled attribute to privatelink endpoint resources and data sources. The attribute allows users to explicitly control whether PSC port-mapping is enabled when creating GCP private endpoints.

Key Changes:

  • Added port_mapping_enabled boolean attribute to privatelink_endpoint (optional, ForceNew) and privatelink_endpoint_service (computed)
  • Implemented test coverage for both explicitly enabled and disabled port mapping scenarios
  • Updated documentation for all affected resources and data sources

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/service/privatelinkendpoint/resource.go Added optional port_mapping_enabled field to resource schema and create/read logic
internal/service/privatelinkendpoint/data_source.go Added computed port_mapping_enabled field to data source schema and read logic
internal/service/privatelinkendpointservice/resource.go Added computed port_mapping_enabled field to resource schema and read logic for GCP
internal/service/privatelinkendpointservice/data_source.go Added computed port_mapping_enabled field to data source schema and read logic for GCP
internal/service/privatelinkendpoint/resource_test.go Added test coverage for port mapping enabled/disabled scenarios and updated existing test assertions
docs/resources/privatelink_endpoint.md Documented the new port_mapping_enabled attribute
docs/resources/privatelink_endpoint_service.md Documented the new port_mapping_enabled attribute
docs/data-sources/privatelink_endpoint.md Documented the new port_mapping_enabled attribute
docs/data-sources/privatelink_endpoint_service.md Documented the new port_mapping_enabled attribute
.changelog/4017.txt Added changelog entries for the enhancement

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

var (
resourceName = "mongodbatlas_privatelink_endpoint.test"
orgID = os.Getenv("MONGODB_ATLAS_ORG_ID")
projectName = "test-acc-tf-p-gcp-port-based-routing-feature-flag-enabled"
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The project name references 'feature-flag-enabled' but this test is for explicitly enabled port mapping, not feature flag testing. Consider renaming to something like 'test-acc-tf-p-gcp-port-mapping-enabled' for clarity.

Suggested change
projectName = "test-acc-tf-p-gcp-port-based-routing-feature-flag-enabled"
projectName = "test-acc-tf-p-gcp-port-mapping-enabled"

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: This will be updated to use the random name once the feature is officially released (GA) and not gated by a feature-flag (before merging to master)

Copy link

@lizo-mdb lizo-mdb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Accepted values are: [AWS regions](https://docs.atlas.mongodb.com/reference/amazon-aws/#amazon-aws), [AZURE regions](https://docs.atlas.mongodb.com/reference/microsoft-azure/#microsoft-azure) and [GCP regions](https://docs.atlas.mongodb.com/reference/google-gcp/#std-label-google-gcp)
* `timeouts`- (Optional) The duration of time to wait for Private Endpoint to be created or deleted. The timeout value is defined by a signed sequence of decimal numbers with a time unit suffix such as: `1h45m`, `300s`, `10m`, etc. The valid time units are: `ns`, `us` (or `µs`), `ms`, `s`, `m`, `h`. The default timeout for Private Endpoint create & delete is `1h`. Learn more about timeouts [here](https://www.terraform.io/plugin/sdkv2/resources/retries-and-customizable-timeouts).
* `delete_on_create_timeout`- (Optional) Indicates whether to delete the resource being created if a timeout is reached when waiting for completion. When set to `true` and timeout occurs, it triggers the deletion and returns immediately without waiting for deletion to complete. When set to `false`, the timeout will not trigger resource deletion. If you suspect a transient error when the value is `true`, wait before retrying to allow resource deletion to finish. Default is `true`.
* `port_mapping_enabled` - (Optional) Flag that indicates whether this endpoint service uses PSC port-mapping.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Is there any example we can update also? I struggle a bit to understand this feature seeing only this variable description

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the description available on Atlas and unfortunately we are not able to update it. (we need DELETE + CREATE)

Copy link
Collaborator

@EspenAlbert EspenAlbert Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just wanted to know if we have an example in repo_root/examples/* .tf that we could update?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes total sense. I will add an example for this.

),
},
{
ResourceName: resourceName,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice to add the import step 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's just git displaying the diff wrong as that was already there but it just got pushed back to the end :)

resource.TestCheckResourceAttrSet(resourceName, "region"),
resource.TestCheckResourceAttr(resourceName, "provider_name", providerName),
resource.TestCheckResourceAttr(resourceName, "region", region),
resource.TestCheckResourceAttr(resourceName, "port_mapping_enabled", "true"),
Copy link
Collaborator

@EspenAlbert EspenAlbert Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] not sure if it is worth testing the attribute changes:
null -> true
true -> false

Also, does this value have a default? (What is returned by the API if the field is not set?)
What is the difference between false and null? Will null -> false and ForceNew leading to an unnecessary DELETE+CREATE plan?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Attribute changes (null -> true, true -> false):
    These transitions were not tested since I thought it is not needed because of ForceNew. I will add those.

  2. Default value:
    Atlas defaults this attribute to false. Should we do the same?

  3. null vs false:
    I will check this as I do not have a response now (I personally expect yes).

Copy link
Collaborator

@oarbusi oarbusi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions
Copy link
Contributor

This PR has gone 7 days without any activity and meets the project’s definition of "stale". This will be auto-closed if there is no new activity over the next 7 days. If the issue is still relevant and active, you can simply comment with a "bump" to keep it open, or add the label "not_stale". Thanks for keeping our repository healthy!

@github-actions github-actions bot added the stale label Dec 29, 2025
@github-actions github-actions bot closed this Jan 1, 2026
@filipcirtog filipcirtog added the not_stale Not stale issue or PR label Jan 8, 2026
@filipcirtog filipcirtog requested a review from Copilot January 13, 2026 11:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@filipcirtog filipcirtog requested a review from Copilot January 13, 2026 11:16
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 3 to 6
azurerm = {
source = "hashicorp/azurerm"
version = "~> 3.0"
}
Copy link

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The azurerm provider is incorrectly included in a GCP example. This should be removed as it's not used in any GCP resources and will cause confusion.

Suggested change
azurerm = {
source = "hashicorp/azurerm"
version = "~> 3.0"
}

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes indeed. changed thx.

resource.TestCheckResourceAttr(resourceName, "port_mapping_enabled", "true"),
),
},
{
Copy link

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test attempts to toggle port_mapping_enabled from true to false, but this field has ForceNew: true which will recreate the resource. This test step verifies recreation behavior rather than update behavior, which may not be the intended test case based on the test structure.

Suggested change
{
{
ResourceName: resourceName,
ImportStateIdFunc: importStateIDFunc(resourceName),
ImportState: true,
ImportStateVerify: true,
},
},
})
}
func TestAccNetworkRSPrivateLinkEndpointGCP_basic_with_new_architecture_explicitly_disabled(t *testing.T) {
var (
resourceName = "mongodbatlas_privatelink_endpoint.test"
orgID = os.Getenv("MONGODB_ATLAS_ORG_ID")
projectName = "test-acc-tf-p-gcp-port-based-routing-feature-flag-disabled"
region = "us-west3"
providerName = "GCP"
)
resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acc.PreCheckBasic(t) },
ProtoV6ProviderFactories: acc.TestAccProviderV6Factories,
CheckDestroy: checkDestroy,
Steps: []resource.TestStep{
{

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test added according to the previous PR review

),
},
{
Config: configWithPortMapping(orgID, projectName, providerName, region, true),
Copy link

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test attempts to toggle port_mapping_enabled from false to true, but this field has ForceNew: true which will recreate the resource. This test step verifies recreation behavior rather than update behavior, which may not be the intended test case based on the test structure.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test added according to the previous PR review


The new PSC port-based architecture simplifies setup by requiring only 1 endpoint instead of 50. Enable it by setting `port_mapping_enabled = true` on the endpoint resource.

**Important:** For the new port-based architecture, the `endpoint_service_id` must match the `endpoint_name` in the `endpoints` block. Although the new API ignores the `endpoint_service_id` value, it is still required by the Terraform provider.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is endpoint_service_id still required just because it was already marked as required in the schema and we don't want to change that or is there some other reason?

  • Did we consider changing it to optional?
  • I am also not sure why endpoint_service_id it must match endpoint_name if it is ignored.

Would not mention: "Although the new API ignores the endpoint_service_id value, it is still required by the Terraform provider."

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the GET API call relies on endpoint_service_id in all cases, we don’t have a direct flag for this (similar to port_mapping_enabled on this resource, which is only computed in this specific case i.e., it’s defined on the endpoint_service, not on the endpoint).

The only viable approach appears to be:

  • Not set → Indicates new architecture
  • Set → Indicates old architecture

Would this approach work for you?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From offline discussion:

  • endpoint_service_id is used by the provider to call the private endpoint APIs: example
  • Could move to optional and if not present, use the endpoint_name

Keeping as is makes sense to me, but I would improve the wording in the documentation as mentioned in the previous comment.
We should bring this topic to the tech sync to get input from the team.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As agreed on the TD. Continue using endpoint_service_id as it is a PK within all providers. As a result, for GCP with port-based architecture, use endpoint_service_id, private_endpoint_ip_address and a new field gcp_endpoint_status.

@@ -0,0 +1,100 @@
# Example with GCP with Port-Based architecture and MongoDB Atlas Private Endpoint

This project demonstrates the **new PSC port-based architecture** for setting up GCP Private Service Connect with MongoDB Atlas, which requires only 1 endpoint.
Copy link
Collaborator

@EspenAlbert EspenAlbert Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] what is PSC?

Seems like the benefits are a bit spread and duplicated across this doc.
Maybe I would start with the benefit to motivate the user instead of leaving it for the end

@filipcirtog filipcirtog changed the title feat: Adds port_mapping_enabled attribute to privatelink_endpoint and privatelink_endpoint_service feat: Adds port_mapping_enabled attribute to privatelink_endpoint and privatelink_endpoint_service Jan 14, 2026
* `endpoint_name` - Forwarding rule that corresponds to the endpoint you created in GCP.
* `ip_address` - Private IP address of the network endpoint group you created in GCP.
* `status` - Status of the endpoint. Atlas returns one of the [values shown above](https://docs.atlas.mongodb.com/reference/api/private-endpoints-endpoint-create-one/#std-label-ref-status-field).
* `port_mapping_enabled` - Flag that indicates whether this endpoint service uses PSC port-mapping.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we add a link from here to "## Example with GCP (Port-Based Architecture)" section?

@@ -0,0 +1,100 @@
# Example with GCP with Port-Based architecture and MongoDB Atlas Private Endpoint
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@filipcirtog we're missing the migration guide from "old" to "new" supported by an example on how to do this for module users.
Is it planned? cc @lantoli

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. a migration guide is planned for a separate PR

* `project_id` - (Required) Unique identifier for the project.
* `private_link_id` - (Required) Unique identifier of the `AWS` or `AZURE` PrivateLink connection which is created by `mongodbatlas_privatelink_endpoint` resource.
* `endpoint_service_id` - (Required) Unique identifier of the interface endpoint you created in your VPC with the `AWS`, `AZURE` or `GCP` resource.
* `endpoint_service_id` - (Required) Unique identifier of the interface endpoint you created in your VPC with the `AWS`, `AZURE` or `GCP` resource. **Note:** For GCP with the new port-based architecture (when `port_mapping_enabled = true` on the endpoint resource), this value must match the `endpoint_name` in the `endpoints` block. Although the new API ignores this value, it is still required by the Terraform provider.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

related to our TD conversation: why don't we make it optional and add a validation error if port_mapping_enabled = false? (probably missing something)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you! this was an initial iteration before flagging the decision, will change it as soon as we commit to one

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As agreed on the TD: Continue using endpoint_service_id as it is a PK within all providers. As a result, for GCP with port-based architecture, use endpoint_service_id, private_endpoint_ip_address and a new field gcp_endpoint_status and not the endpoints array anymore.

@filipcirtog filipcirtog requested a review from Copilot January 22, 2026 18:37
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


| Feature | Legacy Architecture | New Port-Based Architecture |
|---------|-------------------|---------------------------|
| Endpoints Required | up to 100 | 1 |
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The table states 'up to 100' endpoints for legacy architecture, but the PR description and other documentation consistently mentions '50 endpoints'. This inconsistency should be corrected to maintain accuracy across documentation.

Suggested change
| Endpoints Required | up to 100 | 1 |
| Endpoints Required | up to 50 | 1 |

Copilot uses AI. Check for mistakes.
projectID := d.Get("project_id").(string)
privateLinkID := conversion.GetEncodedID(d.Get("private_link_id").(string), "private_link_id")
endpointServiceID := conversion.GetEncodedID(d.Get("endpoint_service_id").(string), "endpoint_service_id")
providerName := d.Get("provider_name").(string)
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The endpoint_service_id is no longer encoded using conversion.GetEncodedID, but other IDs like private_link_id still are. This inconsistency could lead to confusion. Consider adding a comment explaining why endpoint_service_id doesn't need encoding, or ensure consistent handling across all IDs.

Suggested change
providerName := d.Get("provider_name").(string)
providerName := d.Get("provider_name").(string)
// endpoint_service_id is expected by the Atlas API in its original form and therefore
// is not passed through conversion.GetEncodedID, unlike private_link_id above.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed


This example demonstrates the **legacy PSC architecture** which requires 50 endpoints.

For the **new PSC port-based architecture** (which requires only 1 endpoint and is enabled with `port_mapping_enabled = true`), see the [`gcp-port-based`](../gcp-port-based/) example.
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inline code formatting uses backticks for attribute name but the same formatting is not consistently applied to the value 'true'. For consistency, consider formatting it as port_mapping_enabled = true or writing it without code formatting altogether.

Suggested change
For the **new PSC port-based architecture** (which requires only 1 endpoint and is enabled with `port_mapping_enabled = true`), see the [`gcp-port-based`](../gcp-port-based/) example.
For the **new PSC port-based architecture** (which requires only 1 endpoint and is enabled with port_mapping_enabled = true), see the [`gcp-port-based`](../gcp-port-based/) example.

Copilot uses AI. Check for mistakes.
@filipcirtog filipcirtog requested a review from Copilot January 22, 2026 18:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

resource.TestCheckResourceAttr(resourceName, "region", region),
resource.TestCheckResourceAttr(resourceName, "port_mapping_enabled", "false"),
),
},
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test transitions from port_mapping_enabled = false to port_mapping_enabled = true. Since port_mapping_enabled is defined as ForceNew: true in the resource schema, this change will force resource recreation. Consider adding a comment explaining that this step verifies the ForceNew behavior, or split this into separate test cases if the intent is to test independent scenarios.

Suggested change
},
},
// Changing port_mapping_enabled here is intentional: it is marked ForceNew in the schema,
// so this step verifies that toggling it forces recreation of the resource.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. this should be documented.

@filipcirtog filipcirtog marked this pull request as draft January 22, 2026 19:32
@filipcirtog filipcirtog requested a review from Copilot January 22, 2026 19:53
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@filipcirtog filipcirtog requested a review from Copilot January 22, 2026 20:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

return diag.FromErr(fmt.Errorf(errorEndpointSetting, "gcp_status", endpointServiceID, err))
}

if serviceEndpoint.GetPortMappingEnabled() && serviceEndpoint.Endpoints != nil && len(*serviceEndpoint.Endpoints) == 1 {
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conditional logic for determining port-based vs legacy architecture is duplicated between resource.go (lines 318-332) and data_source.go (lines 170-184). Consider extracting this logic into a shared helper function to reduce duplication and ensure consistency.

Copilot uses AI. Check for mistakes.
private_link_id = mongodbatlas_privatelink_endpoint.test.private_link_id
provider_name = "GCP"
endpoint_service_id = google_compute_network.default.name
endpoint_service_id = "the-endpoint-group-name"
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The placeholder value 'the-endpoint-group-name' is not descriptive enough for example code. Consider using a more meaningful placeholder like 'my-endpoint-group' or adding a comment explaining what value should be used here.

Copilot uses AI. Check for mistakes.

locals {
endpoint_service_id = google_compute_network.default.name
endpoint_service_id = "the-endpoint-group-name"
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same placeholder value as line 59. These should match the actual resource reference (like it was before: google_compute_network.default.name) or be clearly documented as placeholders.

Suggested change
endpoint_service_id = "the-endpoint-group-name"
endpoint_service_id = mongodbatlas_privatelink_endpoint_service.test.endpoint_service_id

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement not_stale Not stale issue or PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants